Skip to content

Conversation

ishitatsuyuki
Copy link
Contributor

The state IDs were not remapped, which will usually result in an index out of range error.

@BurntSushi
Copy link
Member

Is it possible to add a regression test?

@ishitatsuyuki
Copy link
Contributor Author

Sure, let me give it a try tomorrow (it's late here today).

@ishitatsuyuki
Copy link
Contributor Author

Hi, I added a test based on examples in Automaton::is_special_state. I'm asserting that the initial state is .is_start_state(), but this seems to be broken for some reason. Could you help figuring out why? I'm afraid I'm not familiar enough with how the start state ranges are being assigned.

@ishitatsuyuki
Copy link
Contributor Author

Alternatively, I could remove the assert for now and file an issue for it to be fixed later.

The state IDs were not remapped, which will usually result in an index
out of range error.

Add a test based on is_special_state's doctest, which will validate the
start state's behavior with a custom searcher.
@ishitatsuyuki
Copy link
Contributor Author

It looks like start states were simply being removed with set_no_special_start_states, so this was the intended behavior. I have removed the assert now. Let me know if the tests look good to you.

@ishitatsuyuki
Copy link
Contributor Author

Hi, mind giving this a review?

@ishitatsuyuki
Copy link
Contributor Author

Friendly ping, let me know if there's something else I need to address

@BurntSushi
Copy link
Member

Thank you! Great work on the test. :-)

BurntSushi pushed a commit that referenced this pull request Oct 9, 2025
The state IDs were not remapped, which will usually result in an index
out of range error.

Add a test based on is_special_state's doctest, which will validate the
start state's behavior with a custom searcher.

Closes #1195
@BurntSushi BurntSushi closed this in bd345d7 Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants